-
Notifications
You must be signed in to change notification settings - Fork 575
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Top level import of templates #1779
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1779 +/- ##
=======================================
Coverage 98.91% 98.91%
=======================================
Files 207 207
Lines 15595 15599 +4
=======================================
+ Hits 15426 15430 +4
Misses 169 169
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking awesome, thank you @rmoyard for it! 😍 It Will be so simple to use templates.
There seem to be a couple of remaining occurrences that require a change:
- In the
docs
folder, there seem to be two occurrences ofqml_templates
left:
else:
link = "code/qml_templates"
-
The pages in
introduction/interfaces
seem to have code examples such asqml.templates.AmplitudeEmbedding
, etc. -
Minor: the https://github.com/PennyLaneAI/pennylane/blob/master/doc/development/adding_templates.rst page also has some occurrences of the full template path (though they render well either way).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome that the CI checks are now in place. 😊 🚀
One other set of replacements we could do is to change the code examples we have in the pennylane
folder. Many would use for example qml.templates.StronglyEntanglingLayers
, etc. Here we could make sure that users see the shorthand syntax.
doc/introduction/templates.rst
Outdated
The template decorator can used to register a quantum function as a template. | ||
|
||
.. autosummary:: | ||
:toctree: | ||
|
||
pennylane.templates.template |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, the template decorator is deprecated, we should probably remove it :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(at the very least, lets remove it from here)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@josh146 should we add a warning when it's used? (Or perhaps we'll be able to simply remove it as likely it was not too widely used).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there is already no warning, we should add a deprecation warning for 0.19 and then remove it in 0.20 🙂
doc/introduction/templates.rst
Outdated
Layering Function | ||
----------------- | ||
|
||
The layer function creates a new template by repeatedly applying a sequence of quantum | ||
gates to a set of wires. You can import this function both via | ||
``qml.layer`` and ``qml.templates.layer``. | ||
|
||
.. autosummary:: | ||
|
||
pennylane.layer | ||
|
||
Utility functions for input checks | ||
---------------------------------- | ||
|
||
.. automodapi:: pennylane.templates.utils | ||
:no-heading: | ||
:include-all-objects: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mariaschuld just double checking here on your thoughts.
Definitely we should remove the utility checks from the quickstart, iirc they are deprecated and should be removed?
The layer function is not deprecated, but I'm wondering if it is worth keeping.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we should remove the utilities :)
No strong feelings about the layering function, does it just implement a for loop? If so, is it important for users? I know that the QAOA module uses it a lot!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, the layer()
function is simply a for loop under the hood:
def layer(template, depth, *args, **kwargs):
for i in range(0, int(depth)):
arg_params = [k[i] for k in args]
template(*arg_params, **kwargs)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! 🙂 Awesome to have this 🚀
Description of the Change:
Previously, templates had to be used by first accessing .templates and then use the specific template name
qml.templates.QFT
, now they are top level imported and can be used directly e.g.qml.QFT
.QFT
is also added back to the list of operations indefault.qubit
anddefault.mixed
, therefore it will use the matrix representation instead of decomposing first.The file
qaoa.py
for qaoa embedding was renamedqaoaembedding.py
because of collusion with the qaoa module.Benefits:
UI is simpler and the user does not need to know about the distinction between operations and templates.